Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: bb.js tests of ClientIVC #9412

Merged
merged 53 commits into from
Nov 8, 2024
Merged

feat: bb.js tests of ClientIVC #9412

merged 53 commits into from
Nov 8, 2024

Conversation

codygunton
Copy link
Contributor

@codygunton codygunton commented Oct 24, 2024

Extend the ivc-integration-tests suite to execute tests through bb.js, but using bb.js as a library and via the browser. When run, these tests give both memory and time logs. We should make these easier to read, but for now they're very useful. Note that, for now, the browser version of the tests are not run in CI because yarn build:browser hangs in that context.

@codygunton codygunton changed the title dontmerge: Cg/browser civc feat: Cg/browser civc Oct 30, 2024
@codygunton codygunton changed the title feat: Cg/browser civc feat: Browser tests of ClientIVC Oct 30, 2024
@codygunton codygunton marked this pull request as ready for review October 31, 2024 14:32
@codygunton codygunton self-assigned this Nov 4, 2024
}
},
{
"name": "wasm-threads-assert",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added asserts build for wasm, much faster to build than dbg. Considered making it the default but idk, might affect performance if we use heavy asserts.

ivc.auto_verify_mode = true;
ivc.trace_structure = TraceStructure::E2E_FULL_TEST;

// Accumulate the entire program stack into the IVC
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just carrying along this todo bc this second was basically a copy and paste from main.cpp.

@@ -545,6 +545,18 @@ export class BarretenbergApi {
return out as any;
}

async acirProveAndVerifyAztecClient(acirVec: Uint8Array[], witnessVec: Uint8Array[]): Promise<boolean> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need separate prove and verify flows yet. My goal is to get something measurable, someone can easily follow the pattern to add these in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also we don't ever want a verify flow through wasm. Prob we should star to cut stuff out of the wasm binary since it's already large.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you add this by hand? Just checking as this file is code generated by the bindgen util.

@@ -0,0 +1,17 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI the browser test does not need a running server to execute. But I do add a a webpage as in browser test app to serve the app for execution in a browser bc I want to be able to use the browser's built in profiling and debugging tools.

@@ -1,13 +1,7 @@
import { BB_RESULT, executeBbClientIvcProof, verifyClientIvcProof } from '@aztec/bb-prover';
import { ClientIvcProof } from '@aztec/circuits.js';
import { createDebugLogger } from '@aztec/foundation/log';

import { jest } from '@jest/globals';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are now three tests that tests the same circuits: the native version, this 'wasm' version that uses bb.js as a library' and a browser version that executes through playwright. Prob we don't need all three, your call.

@@ -691,6 +691,7 @@ __metadata:
resolution: "@aztec/ivc-integration@workspace:ivc-integration"
dependencies:
"@aztec/bb-prover": "workspace:^"
"@aztec/bb.js": ../../ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk how to not have a huge diff here.

@codygunton codygunton requested a review from charlielye November 5, 2024 02:37
@@ -545,6 +545,18 @@ export class BarretenbergApi {
return out as any;
}

async acirProveAndVerifyAztecClient(acirVec: Uint8Array[], witnessVec: Uint8Array[]): Promise<boolean> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you add this by hand? Just checking as this file is code generated by the bindgen util.

const [bytecodes, witnessStack] = await generate6FunctionTestingIVCStack();

logger(`calling prove and verify...`);
const verifyResult = await proveAndVerifyAztecClient(page, bytecodes, witnessStack);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this just executes the exposed function in Node - so I don't think this is actually executing in the browser.
That's why I had an actual server serve a page in the acir_tests.

I can see you do actually have a webpack bundled page below - so I think you just need to change this test to load that page (assuming served on e.g. localhost:8080) and then evaluate a call that clicks the button (or calls the function in the page exposed on the window, think thats what i did in my acir_tests).

I guess the faff from CI perspective is you need a wrapper script that does the webpack serve or whatever to start the server, before running the jest test.

@codygunton codygunton changed the title feat: Browser tests of ClientIVC feat: bb.js tests of ClientIVC Nov 8, 2024
@codygunton codygunton enabled auto-merge (squash) November 8, 2024 17:44
@codygunton codygunton merged commit 90696cd into master Nov 8, 2024
74 checks passed
@codygunton codygunton deleted the cg/browser-civc branch November 8, 2024 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants